Define Python Prelude in Laurel instead of Core#506
Define Python Prelude in Laurel instead of Core#506keyboardDrummer wants to merge 50 commits intomainfrom
Conversation
|
We will use the "PythonLaurelCorePrelude" in this PR #489 for Python-> Laurel translation instead of the "CorePrelude". |
Yes, this PR conflicts with that one, but I think that's OK we can resolve the conflicts. I don't mind resolving them in whatever PR gets merged last. |
Suggestion: Add test cases for quantifier triggersThe PR adds trigger support to Consider adding trigger test cases to This would exercise the full pipeline: parsing ( |
Suggestion: Add test case for unsafe destructor resolutionThe PR registers unsafe destructors ( _ ← defineName p.name (.parameter p) (some $ dt.name.text ++ ".." ++ p.name.text ++ "!")And the Laurel prelude uses them, e.g. in However, no Laurel-level test directly exercises this syntax. The new This would verify that the |
| | .Forall p trigger b => do return ⟨.Forall p (← trigger.attach.mapM (fun pv => have := pv.property; rewriteTypeHierarchyExpr pv.val)) (← rewriteTypeHierarchyExpr b), md⟩ | ||
| | .Exists p trigger b => do return ⟨.Exists p (← trigger.attach.mapM (fun pv => have := pv.property; rewriteTypeHierarchyExpr pv.val)) (← rewriteTypeHierarchyExpr b), md⟩ |
There was a problem hiding this comment.
These lines are 172 chars each (hard limit is 100). Extracting the trigger transformation into a let binding also gives Lean the membership proof via destructuring, which means no explicit decreasing_by clause is needed for the trigger.
| | .Forall p trigger b => do return ⟨.Forall p (← trigger.attach.mapM (fun pv => have := pv.property; rewriteTypeHierarchyExpr pv.val)) (← rewriteTypeHierarchyExpr b), md⟩ | |
| | .Exists p trigger b => do return ⟨.Exists p (← trigger.attach.mapM (fun pv => have := pv.property; rewriteTypeHierarchyExpr pv.val)) (← rewriteTypeHierarchyExpr b), md⟩ | |
| | .Forall p trigger b => do | |
| let trigger' ← trigger.attach.mapM fun ⟨t, _⟩ => rewriteTypeHierarchyExpr t | |
| return ⟨.Forall p trigger' (← rewriteTypeHierarchyExpr b), md⟩ | |
| | .Exists p trigger b => do | |
| let trigger' ← trigger.attach.mapM fun ⟨t, _⟩ => rewriteTypeHierarchyExpr t | |
| return ⟨.Exists p trigger' (← rewriteTypeHierarchyExpr b), md⟩ |
Suggestion: Simplify
|
Request: Track disabled procedure multiple-requires testIn // This test fails because Core incorrectly report error locations on procedure preconditions
// procedure multipleRequiresCaller() {
// var a: int := multipleRequires(1, 2);
// var b: int := multipleRequires(-1, 2);
// error: assertion does not hold
// }Could you create a tracking issue for the Core error location bug so this doesn't get forgotten? The function version ( |
Question: Is the Laurel pipeline's use of
|
|
Per the caveat, the DDM is designed to resolve types to variables as part of its typechecking and type inference. If it doesn't do that, then tradeoff is lack of type inference in polymorphic functions. It sounds like you don't care about that feature, but it's definitely a feature that was added intentionally by request. |
…thonLaurelCorePrelude.lean to include new definitions
Tracked in Asana but sadly not in GH. How do you think we should approach this? Should it be duplicated, only in GH, or is only Asana OK? |
It's not a feature that I'm using, and I'm not sure how I would use it. It looks like it performs checks during parsing, but generally the resolution information is not maintained after that. Laurel needs the reference=>definition map for its phases, and it needs to rerun resolution after each phase. What's your intention of how DDM resolution features should be used? Currently Laurel's resolution logic is specified in Resolution.lean. I expect that from the semantic definition of Laurel, we'll be able to extract the same logic so we won't need much of the contents of Resolution.lean any more. It would then be duplicative to also specify resolution logic in the DDM specification of Laurel. My feeling is that it's good to keep the syntactic and semantic definition of a language separate, so I'm not optimistic about using resolution features of the DDM. What are your thoughts on that? |
|
Thanks for the excellent review! I've applied the changes to the two PRs replacing this one. |
Putting this in draft for now, because my impression is that since the current PySpecs are translated to Core instead of Python or Laurel, it is not possible to reliably turn on resolution error reporting for Laurel when using Python.
This PR turns on resolution error reporting in Laurel, and works towards removing the need for the
TCoretype in Laurel. It conflicts with changes from another in-progress PR, #489. I think that one will need more time to be ready for merging, so I think it makes sense to review/merge this one first. I will happily resolve conflicts in 489 after this is merged.Changes
CorePrelude.leanwithPythonRuntimeLaurelPart.leanandPythonRuntimeCorePart.lean. The CorePart is needed because Python requires axioms in Core and Laurel does not support axioms. I think we should not add axioms to Laurel. A long term solution would be that the Python pipeline stops relying on axioms.--updateoption to run_py_analyze.sh to update the test expect files (the bytecode offsets in them change often)Caveats
PythonRuntimeCorePart.leanneeds a lot of type definitions that are already provided inPythonPreludeInLaurel.lean. Ideally we'd find a way so that's no longer necessary, using something like#strata_parsing_only. @joehendrix is that something we could add? I looked into adding that but it wasn't so simple.Tested
run_py_analyze.sh